Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minor update: dissipation scaling between translation and rotation #392

Draft
wants to merge 3 commits into
base: update-0.3.3
Choose a base branch
from

Conversation

skim0119
Copy link
Collaborator

@skim0119 skim0119 added the discussion Topic that needs to be discussed. label Jun 12, 2024
@skim0119 skim0119 self-assigned this Jun 12, 2024
@sy-cui
Copy link
Contributor

sy-cui commented Jun 26, 2024

Couple of points here:

  • Static cases like axial stretching and timoshenko are okay. The analytical results are recovered
  • Catenary case blows up with the current parameter. Might need to adjust the damping constant
  • Snake and flagella cases are broken due to some issue in base_system (e.g. run the snake case to replicate the issue). It does not seem to be related to this PR.
  • The new implementation is okay, but it erases the original behavior and may break user code after the update is released. I thought the plan was to keep the original behavior but add a deprecation warning?

@skim0119
Copy link
Collaborator Author

Catenary case blows up with the current parameter. Might need to adjust the damping constant

@Ali-7800 Do you remember how you tuned the damping coefficient for Catenary case before?

  • Snake and flagella cases are broken due to some issue in base_system (e.g. run the snake case to replicate the issue). It does not seem to be related to this PR.

Those cases will be fixed with #367

  • The new implementation is okay, but it erases the original behavior and may break user code after the update is released. I thought the plan was to keep the original behavior but add a deprecation warning?

We can do that if we consider this change as an update. I think we can make a hotfix if we consider this change a bug fix. @armantekinalp opinion?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Topic that needs to be discussed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants